pkg/services/annotations/annotationsimpl/cleanup_test.go (1)

237-271: Manual ID assignment couples test setup to DB internals.

Line 241 sets ID: int64(i + 1), which relies on the DB accepting explicitly provided IDs and not conflicting with auto-increment sequences. This works for SQLite but may break if the test DB engine changes or if rows from a previous subtest leave the auto-increment counter advanced (though DELETE doesn't reset it). Since the IDs are also baked into newAnnotationTags at line 270, any mismatch would silently produce orphaned tags.

Consider either removing explicit ID assignment and querying back for inserted IDs, or documenting this assumption.

pkg/services/annotations/annotationsimpl/xorm_store.go (2)

536-538: Variable names x, y are cryptic for returned values.

x, y don't convey meaning. Use affected, err (or batchAffected, batchErr to avoid shadowing) for readability.

Example fix for one occurrence

-			x, y := r.deleteByIDs(ctx, "annotation", ids)
-			r.log.Error("cleaned annotations by time", "count", len(ids), "affected", x, "err", y)
-			return x, y
+			batchAffected, batchErr := r.deleteByIDs(ctx, "annotation", ids)
+			r.log.Debug("cleaned annotations by time", "count", len(ids), "affected", batchAffected, "err", batchErr)
+			return batchAffected, batchErr

Also applies to: 556-558, 578-580

---

605-618: SQLite ID-inlining branch uses O(n²) string concatenation.

Lines 609–612 build the ID list via repeated fmt.Sprintf("%s, %d", ...), which creates a new string allocation on every iteration. With the test batch size of 32767, this becomes expensive. Use strings.Builder or strings.Join instead.

Also, the condition at line 608 checks r.cfg.AnnotationCleanupJobBatchSize > sqliteParameterLimit (the configured maximum), not len(ids) (the actual count). This means even a batch of 5 IDs would take the inlining path if the configured batch size exceeds 999.

Proposed fix using strings.Builder

-	if r.db.GetDBType() == migrator.SQLite && r.cfg.AnnotationCleanupJobBatchSize > sqliteParameterLimit {
-		values := fmt.Sprint(ids[0])
-		for _, v := range ids[1:] {
-			values = fmt.Sprintf("%s, %d", values, v)
-		}
-		sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, values)
+	if r.db.GetDBType() == migrator.SQLite && len(ids) > sqliteParameterLimit {
+		var b strings.Builder
+		b.WriteString(strconv.FormatInt(ids[0], 10))
+		for _, v := range ids[1:] {
+			b.WriteByte(',')
+			b.WriteString(strconv.FormatInt(v, 10))
+		}
+		sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, b.String())